All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to process hvsock connection
@ 2015-07-28 12:35 ` Dexuan Cui
  0 siblings, 0 replies; 18+ messages in thread
From: Dexuan Cui @ 2015-07-28 12:35 UTC (permalink / raw)
  To: gregkh, davem, stephen, netdev, linux-kernel, driverdev-devel,
	olaf, apw, jasowang, kys
  Cc: pebolle, stefanha, vkuznets, dan.carpenter

With the 2 APIs supplied by the VMBus driver, the coming net/hvsock driver
can register 2 callbacks and can know when a new hvsock connection is
offered by the host, and when a hvsock connection is being closed by the
host.

Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
 drivers/hv/Makefile           |  4 ++-
 drivers/hv/channel_mgmt.c     |  9 ++++++
 drivers/hv/hvsock_callbacks.c | 71 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/hyperv.h        | 10 ++++++
 4 files changed, 93 insertions(+), 1 deletion(-)
 create mode 100644 drivers/hv/hvsock_callbacks.c

diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
index 39c9b2c..ef6f8a8 100644
--- a/drivers/hv/Makefile
+++ b/drivers/hv/Makefile
@@ -4,5 +4,7 @@ obj-$(CONFIG_HYPERV_BALLOON)	+= hv_balloon.o
 
 hv_vmbus-y := vmbus_drv.o \
 		 hv.o connection.o channel.o \
-		 channel_mgmt.o ring_buffer.o
+		 channel_mgmt.o ring_buffer.o \
+		 hvsock_callbacks.o
+
 hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o hv_fcopy.o hv_utils_transport.o
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 7018c53..a8b1e61 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -300,6 +300,12 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
 		return;
 	}
 
+	if (is_hvsock_channel(newchannel)) {
+		if (hvsock_process_offer(newchannel) != 0)
+			goto err_deq_chan;
+		return;
+	}
+
 	/*
 	 * Start the process of binding this offer to the driver
 	 * We need to set the DeviceObject field before calling
@@ -564,7 +570,10 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
 			vmbus_device_unregister(channel->device_obj);
 			put_device(dev);
 		}
+	} else if (is_hvsock_channel(channel)) {
+		hvsock_process_offer_rescind(channel);
 	} else {
+		/* it is a sub-channel. */
 		hv_process_channel_removal(channel,
 			channel->offermsg.child_relid);
 	}
diff --git a/drivers/hv/hvsock_callbacks.c b/drivers/hv/hvsock_callbacks.c
new file mode 100644
index 0000000..28f7b75
--- /dev/null
+++ b/drivers/hv/hvsock_callbacks.c
@@ -0,0 +1,71 @@
+/*
+ * Copyright (c) 2015, Microsoft Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/hyperv.h>
+
+/* We should hold the mutex when getting/setting the function pointers */
+static DEFINE_MUTEX(hvsock_cb_mutex);
+static int (*__process_offer)(struct vmbus_channel *channel);
+static void (*__process_offer_rescind)(struct vmbus_channel *channel);
+
+int hvsock_process_offer(struct vmbus_channel *channel)
+{
+	int ret = -ENODEV;
+
+	mutex_lock(&hvsock_cb_mutex);
+
+	if (__process_offer != NULL)
+		ret = __process_offer(channel);
+
+	mutex_unlock(&hvsock_cb_mutex);
+
+	return ret;
+}
+
+void hvsock_process_offer_rescind(struct vmbus_channel *channel)
+{
+	mutex_lock(&hvsock_cb_mutex);
+
+	if (__process_offer_rescind != NULL)
+		__process_offer_rescind(channel);
+	else
+		hv_process_channel_removal(channel,
+			channel->offermsg.child_relid);
+
+	mutex_unlock(&hvsock_cb_mutex);
+}
+
+void vmbus_register_hvsock_callbacks(
+	int (*process_offer)(struct vmbus_channel *),
+	void (*process_offer_rescind)(struct vmbus_channel *))
+{
+	mutex_lock(&hvsock_cb_mutex);
+
+	__process_offer = process_offer;
+	__process_offer_rescind = process_offer_rescind;
+
+	mutex_unlock(&hvsock_cb_mutex);
+}
+EXPORT_SYMBOL_GPL(vmbus_register_hvsock_callbacks);
+
+void vmbus_unregister_hvsock_callbacks(void)
+{
+	mutex_lock(&hvsock_cb_mutex);
+
+	__process_offer = NULL;
+	__process_offer_rescind = NULL;
+
+	mutex_unlock(&hvsock_cb_mutex);
+}
+EXPORT_SYMBOL_GPL(vmbus_unregister_hvsock_callbacks);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index c8e27da..fda9790 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1269,6 +1269,16 @@ extern __u32 vmbus_proto_version;
 
 int vmbus_send_tl_connect_request(const uuid_le *shv_guest_servie_id,
 				  const uuid_le *shv_host_servie_id);
+
+extern int hvsock_process_offer(struct vmbus_channel *channel);
+extern void hvsock_process_offer_rescind(struct vmbus_channel *channel);
+
+extern void vmbus_register_hvsock_callbacks(
+	int (*process_offer)(struct vmbus_channel *),
+	void (*process_offer_rescind)(struct vmbus_channel *));
+
+void vmbus_unregister_hvsock_callbacks(void);
+
 struct vmpipe_proto_header {
 	u32 pkt_type;
 
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to process hvsock connection
@ 2015-07-28 12:35 ` Dexuan Cui
  0 siblings, 0 replies; 18+ messages in thread
From: Dexuan Cui @ 2015-07-28 12:35 UTC (permalink / raw)
  To: gregkh, davem, stephen, netdev, linux-kernel, driverdev-devel,
	olaf, apw, jasowang, kys
  Cc: pebolle, dan.carpenter, stefanha

With the 2 APIs supplied by the VMBus driver, the coming net/hvsock driver
can register 2 callbacks and can know when a new hvsock connection is
offered by the host, and when a hvsock connection is being closed by the
host.

Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
 drivers/hv/Makefile           |  4 ++-
 drivers/hv/channel_mgmt.c     |  9 ++++++
 drivers/hv/hvsock_callbacks.c | 71 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/hyperv.h        | 10 ++++++
 4 files changed, 93 insertions(+), 1 deletion(-)
 create mode 100644 drivers/hv/hvsock_callbacks.c

diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
index 39c9b2c..ef6f8a8 100644
--- a/drivers/hv/Makefile
+++ b/drivers/hv/Makefile
@@ -4,5 +4,7 @@ obj-$(CONFIG_HYPERV_BALLOON)	+= hv_balloon.o
 
 hv_vmbus-y := vmbus_drv.o \
 		 hv.o connection.o channel.o \
-		 channel_mgmt.o ring_buffer.o
+		 channel_mgmt.o ring_buffer.o \
+		 hvsock_callbacks.o
+
 hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o hv_fcopy.o hv_utils_transport.o
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 7018c53..a8b1e61 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -300,6 +300,12 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
 		return;
 	}
 
+	if (is_hvsock_channel(newchannel)) {
+		if (hvsock_process_offer(newchannel) != 0)
+			goto err_deq_chan;
+		return;
+	}
+
 	/*
 	 * Start the process of binding this offer to the driver
 	 * We need to set the DeviceObject field before calling
@@ -564,7 +570,10 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
 			vmbus_device_unregister(channel->device_obj);
 			put_device(dev);
 		}
+	} else if (is_hvsock_channel(channel)) {
+		hvsock_process_offer_rescind(channel);
 	} else {
+		/* it is a sub-channel. */
 		hv_process_channel_removal(channel,
 			channel->offermsg.child_relid);
 	}
diff --git a/drivers/hv/hvsock_callbacks.c b/drivers/hv/hvsock_callbacks.c
new file mode 100644
index 0000000..28f7b75
--- /dev/null
+++ b/drivers/hv/hvsock_callbacks.c
@@ -0,0 +1,71 @@
+/*
+ * Copyright (c) 2015, Microsoft Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/hyperv.h>
+
+/* We should hold the mutex when getting/setting the function pointers */
+static DEFINE_MUTEX(hvsock_cb_mutex);
+static int (*__process_offer)(struct vmbus_channel *channel);
+static void (*__process_offer_rescind)(struct vmbus_channel *channel);
+
+int hvsock_process_offer(struct vmbus_channel *channel)
+{
+	int ret = -ENODEV;
+
+	mutex_lock(&hvsock_cb_mutex);
+
+	if (__process_offer != NULL)
+		ret = __process_offer(channel);
+
+	mutex_unlock(&hvsock_cb_mutex);
+
+	return ret;
+}
+
+void hvsock_process_offer_rescind(struct vmbus_channel *channel)
+{
+	mutex_lock(&hvsock_cb_mutex);
+
+	if (__process_offer_rescind != NULL)
+		__process_offer_rescind(channel);
+	else
+		hv_process_channel_removal(channel,
+			channel->offermsg.child_relid);
+
+	mutex_unlock(&hvsock_cb_mutex);
+}
+
+void vmbus_register_hvsock_callbacks(
+	int (*process_offer)(struct vmbus_channel *),
+	void (*process_offer_rescind)(struct vmbus_channel *))
+{
+	mutex_lock(&hvsock_cb_mutex);
+
+	__process_offer = process_offer;
+	__process_offer_rescind = process_offer_rescind;
+
+	mutex_unlock(&hvsock_cb_mutex);
+}
+EXPORT_SYMBOL_GPL(vmbus_register_hvsock_callbacks);
+
+void vmbus_unregister_hvsock_callbacks(void)
+{
+	mutex_lock(&hvsock_cb_mutex);
+
+	__process_offer = NULL;
+	__process_offer_rescind = NULL;
+
+	mutex_unlock(&hvsock_cb_mutex);
+}
+EXPORT_SYMBOL_GPL(vmbus_unregister_hvsock_callbacks);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index c8e27da..fda9790 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1269,6 +1269,16 @@ extern __u32 vmbus_proto_version;
 
 int vmbus_send_tl_connect_request(const uuid_le *shv_guest_servie_id,
 				  const uuid_le *shv_host_servie_id);
+
+extern int hvsock_process_offer(struct vmbus_channel *channel);
+extern void hvsock_process_offer_rescind(struct vmbus_channel *channel);
+
+extern void vmbus_register_hvsock_callbacks(
+	int (*process_offer)(struct vmbus_channel *),
+	void (*process_offer_rescind)(struct vmbus_channel *));
+
+void vmbus_unregister_hvsock_callbacks(void);
+
 struct vmpipe_proto_header {
 	u32 pkt_type;
 
-- 
2.1.0

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to process hvsock connection
  2015-07-28 12:35 ` Dexuan Cui
@ 2015-07-29 22:26   ` David Miller
  -1 siblings, 0 replies; 18+ messages in thread
From: David Miller @ 2015-07-29 22:26 UTC (permalink / raw)
  To: decui
  Cc: gregkh, stephen, netdev, linux-kernel, driverdev-devel, olaf,
	apw, jasowang, kys, pebolle, stefanha, vkuznets, dan.carpenter

From: Dexuan Cui <decui@microsoft.com>
Date: Tue, 28 Jul 2015 05:35:11 -0700

> With the 2 APIs supplied by the VMBus driver, the coming net/hvsock driver
> can register 2 callbacks and can know when a new hvsock connection is
> offered by the host, and when a hvsock connection is being closed by the
> host.
> 
> Signed-off-by: Dexuan Cui <decui@microsoft.com>

This is an extremely terrible interface.

It's an opaque hook that allows on registry, and it's solve purpose
is to allow a backdoor call into a foreign driver in another module.

These are exactly the things we try to avoid.

Why not create a real abstraction where clients register an object,
that can be contained as a sub-member inside of their own driver
private, that provides the callback registry mechanism.

That way you can register multiple clients, do things like allow
AF_PACKET capturing of vmbus traffic, etc.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to process hvsock connection
@ 2015-07-29 22:26   ` David Miller
  0 siblings, 0 replies; 18+ messages in thread
From: David Miller @ 2015-07-29 22:26 UTC (permalink / raw)
  To: decui
  Cc: olaf, gregkh, jasowang, driverdev-devel, linux-kernel, stephen,
	stefanha, netdev, apw, pebolle, dan.carpenter

From: Dexuan Cui <decui@microsoft.com>
Date: Tue, 28 Jul 2015 05:35:11 -0700

> With the 2 APIs supplied by the VMBus driver, the coming net/hvsock driver
> can register 2 callbacks and can know when a new hvsock connection is
> offered by the host, and when a hvsock connection is being closed by the
> host.
> 
> Signed-off-by: Dexuan Cui <decui@microsoft.com>

This is an extremely terrible interface.

It's an opaque hook that allows on registry, and it's solve purpose
is to allow a backdoor call into a foreign driver in another module.

These are exactly the things we try to avoid.

Why not create a real abstraction where clients register an object,
that can be contained as a sub-member inside of their own driver
private, that provides the callback registry mechanism.

That way you can register multiple clients, do things like allow
AF_PACKET capturing of vmbus traffic, etc.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to process hvsock connection
  2015-07-29 22:26   ` David Miller
  (?)
@ 2015-07-30 10:20     ` Dexuan Cui
  -1 siblings, 0 replies; 18+ messages in thread
From: Dexuan Cui @ 2015-07-30 10:20 UTC (permalink / raw)
  To: David Miller, KY Srinivasan
  Cc: gregkh, stephen, netdev, linux-kernel, driverdev-devel, olaf,
	apw, jasowang, pebolle, stefanha, vkuznets, dan.carpenter

> From: David Miller
> Sent: Thursday, July 30, 2015 6:27
> 
> From: Dexuan Cui
> Date: Tue, 28 Jul 2015 05:35:11 -0700
> 
> > With the 2 APIs supplied by the VMBus driver, the coming net/hvsock driver
> > can register 2 callbacks and can know when a new hvsock connection is
> > offered by the host, and when a hvsock connection is being closed by the
> > host.
> >
> This is an extremely terrible interface.
> 
> It's an opaque hook that allows on registry, and it's solve purpose
> is to allow a backdoor call into a foreign driver in another module.
> 
> These are exactly the things we try to avoid.

Hi David,
Thanks a lot for your reviewing and the suggestion!

> Why not create a real abstraction where clients register an object,
> that can be contained as a sub-member inside of their own driver
> private, that provides the callback registry mechanism.

Please pardon me for my inexperience.
Can you please be a bit more specific?
I guess maybe you're referencing a common design pattern in the driver
code, so an example in some existing driver would be the best. :-)

"clients register an object " -- 
does the "clients" mean the hvsock driver?
and the "object" means the 2 callbacks?

IMHO, here the vmbus driver has to synchronously pass the 2 events
to the hvsock driver, so a "backdoor call into the hvsock driver" is
inevitable anyway?

e.g., in the path vmbus_process_offer() -> hvsock_process_offer(), the
return value of the latter is important to the former, because on error
the former needs to clean up some internal states of the vmbus driver (that
is, the "goto err_deq_chan").

 
> That way you can register multiple clients, do things like allow
> AF_PACKET capturing of vmbus traffic, etc.

I thought AF_PACKET can only capture IP packets	or Ethernet frames.
Can it be used to capture AF_UNIX packet? 
If yes, I suppose we can consider making it work for AF_HYPERV too,
if people ask for that.

Thanks,
-- Dexuan

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to process hvsock connection
@ 2015-07-30 10:20     ` Dexuan Cui
  0 siblings, 0 replies; 18+ messages in thread
From: Dexuan Cui @ 2015-07-30 10:20 UTC (permalink / raw)
  To: David Miller, KY Srinivasan
  Cc: olaf, gregkh, jasowang, driverdev-devel, linux-kernel, stephen,
	stefanha, netdev, apw, pebolle, dan.carpenter

> From: David Miller
> Sent: Thursday, July 30, 2015 6:27
> 
> From: Dexuan Cui
> Date: Tue, 28 Jul 2015 05:35:11 -0700
> 
> > With the 2 APIs supplied by the VMBus driver, the coming net/hvsock driver
> > can register 2 callbacks and can know when a new hvsock connection is
> > offered by the host, and when a hvsock connection is being closed by the
> > host.
> >
> This is an extremely terrible interface.
> 
> It's an opaque hook that allows on registry, and it's solve purpose
> is to allow a backdoor call into a foreign driver in another module.
> 
> These are exactly the things we try to avoid.

Hi David,
Thanks a lot for your reviewing and the suggestion!

> Why not create a real abstraction where clients register an object,
> that can be contained as a sub-member inside of their own driver
> private, that provides the callback registry mechanism.

Please pardon me for my inexperience.
Can you please be a bit more specific?
I guess maybe you're referencing a common design pattern in the driver
code, so an example in some existing driver would be the best. :-)

"clients register an object " -- 
does the "clients" mean the hvsock driver?
and the "object" means the 2 callbacks?

IMHO, here the vmbus driver has to synchronously pass the 2 events
to the hvsock driver, so a "backdoor call into the hvsock driver" is
inevitable anyway?

e.g., in the path vmbus_process_offer() -> hvsock_process_offer(), the
return value of the latter is important to the former, because on error
the former needs to clean up some internal states of the vmbus driver (that
is, the "goto err_deq_chan").

 
> That way you can register multiple clients, do things like allow
> AF_PACKET capturing of vmbus traffic, etc.

I thought AF_PACKET can only capture IP packets	or Ethernet frames.
Can it be used to capture AF_UNIX packet? 
If yes, I suppose we can consider making it work for AF_HYPERV too,
if people ask for that.

Thanks,
-- Dexuan

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to process hvsock connection
@ 2015-07-30 10:20     ` Dexuan Cui
  0 siblings, 0 replies; 18+ messages in thread
From: Dexuan Cui @ 2015-07-30 10:20 UTC (permalink / raw)
  To: David Miller, KY Srinivasan
  Cc: olaf, gregkh, jasowang, driverdev-devel, linux-kernel, stephen,
	stefanha, netdev, apw, pebolle, dan.carpenter

> From: David Miller
> Sent: Thursday, July 30, 2015 6:27
> 
> From: Dexuan Cui
> Date: Tue, 28 Jul 2015 05:35:11 -0700
> 
> > With the 2 APIs supplied by the VMBus driver, the coming net/hvsock driver
> > can register 2 callbacks and can know when a new hvsock connection is
> > offered by the host, and when a hvsock connection is being closed by the
> > host.
> >
> This is an extremely terrible interface.
> 
> It's an opaque hook that allows on registry, and it's solve purpose
> is to allow a backdoor call into a foreign driver in another module.
> 
> These are exactly the things we try to avoid.

Hi David,
Thanks a lot for your reviewing and the suggestion!

> Why not create a real abstraction where clients register an object,
> that can be contained as a sub-member inside of their own driver
> private, that provides the callback registry mechanism.

Please pardon me for my inexperience.
Can you please be a bit more specific?
I guess maybe you're referencing a common design pattern in the driver
code, so an example in some existing driver would be the best. :-)

"clients register an object " -- 
does the "clients" mean the hvsock driver?
and the "object" means the 2 callbacks?

IMHO, here the vmbus driver has to synchronously pass the 2 events
to the hvsock driver, so a "backdoor call into the hvsock driver" is
inevitable anyway?

e.g., in the path vmbus_process_offer() -> hvsock_process_offer(), the
return value of the latter is important to the former, because on error
the former needs to clean up some internal states of the vmbus driver (that
is, the "goto err_deq_chan").

 
> That way you can register multiple clients, do things like allow
> AF_PACKET capturing of vmbus traffic, etc.

I thought AF_PACKET can only capture IP packets	or Ethernet frames.
Can it be used to capture AF_UNIX packet? 
If yes, I suppose we can consider making it work for AF_HYPERV too,
if people ask for that.

Thanks,
-- Dexuan
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to process hvsock connection
  2015-07-30 10:20     ` Dexuan Cui
  (?)
@ 2015-08-06  4:53       ` Dexuan Cui
  -1 siblings, 0 replies; 18+ messages in thread
From: Dexuan Cui @ 2015-08-06  4:53 UTC (permalink / raw)
  To: David Miller, KY Srinivasan
  Cc: olaf, gregkh, jasowang, driverdev-devel, linux-kernel, stephen,
	stefanha, netdev, apw, pebolle, dan.carpenter

> From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On Behalf
> Of Dexuan Cui
> Sent: Thursday, July 30, 2015 18:20
> To: David Miller <davem@davemloft.net>; KY Srinivasan <kys@microsoft.com>
> Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> stephen@networkplumber.org; stefanha@redhat.com; netdev@vger.kernel.org;
> apw@canonical.com; pebolle@tiscali.nl; dan.carpenter@oracle.com
> Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to
> process hvsock connection
> 
> > From: David Miller
> > Sent: Thursday, July 30, 2015 6:27
> >
> > From: Dexuan Cui
> > Date: Tue, 28 Jul 2015 05:35:11 -0700
> >
> > > With the 2 APIs supplied by the VMBus driver, the coming net/hvsock driver
> > > can register 2 callbacks and can know when a new hvsock connection is
> > > offered by the host, and when a hvsock connection is being closed by the
> > > host.
> > >
> > This is an extremely terrible interface.
> >
> > It's an opaque hook that allows on registry, and it's solve purpose
> > is to allow a backdoor call into a foreign driver in another module.
> >
> > These are exactly the things we try to avoid.
> 
> Hi David,
> Thanks a lot for your reviewing and the suggestion!
> 
> > Why not create a real abstraction where clients register an object,
> > that can be contained as a sub-member inside of their own driver
> > private, that provides the callback registry mechanism.

Hi David,
Can you please have a look at my below questions?

I like your idea of a real abstraction. Your answer would definitely
help me to implement that correctly. 

> Please pardon me for my inexperience.
> Can you please be a bit more specific?
> I guess maybe you're referencing a common design pattern in the driver
> code, so an example in some existing driver would be the best. :-)
> 
> "clients register an object " --
> does the "clients" mean the hvsock driver?
> and the "object" means the 2 callbacks?
> 
> IMHO, here the vmbus driver has to synchronously pass the 2 events
> to the hvsock driver, so a "backdoor call into the hvsock driver" is
> inevitable anyway?
> 
> e.g., in the path vmbus_process_offer() -> hvsock_process_offer(), the
> return value of the latter is important to the former, because on error
> the former needs to clean up some internal states of the vmbus driver (that
> is, the "goto err_deq_chan").
> 
> 
> > That way you can register multiple clients, do things like allow
> > AF_PACKET capturing of vmbus traffic, etc.
> 
> I thought AF_PACKET can only capture IP packets	or Ethernet frames.
> Can it be used to capture AF_UNIX packet?
> If yes, I suppose we can consider making it work for AF_HYPERV too,
> if people ask for that.
> 
> -- Dexuan

Thanks,
-- Dexuan

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to process hvsock connection
@ 2015-08-06  4:53       ` Dexuan Cui
  0 siblings, 0 replies; 18+ messages in thread
From: Dexuan Cui @ 2015-08-06  4:53 UTC (permalink / raw)
  To: David Miller, KY Srinivasan
  Cc: olaf, gregkh, jasowang, driverdev-devel, linux-kernel, stephen,
	stefanha, netdev, apw, pebolle, dan.carpenter

> From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On Behalf
> Of Dexuan Cui
> Sent: Thursday, July 30, 2015 18:20
> To: David Miller <davem@davemloft.net>; KY Srinivasan <kys@microsoft.com>
> Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> stephen@networkplumber.org; stefanha@redhat.com; netdev@vger.kernel.org;
> apw@canonical.com; pebolle@tiscali.nl; dan.carpenter@oracle.com
> Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to
> process hvsock connection
> 
> > From: David Miller
> > Sent: Thursday, July 30, 2015 6:27
> >
> > From: Dexuan Cui
> > Date: Tue, 28 Jul 2015 05:35:11 -0700
> >
> > > With the 2 APIs supplied by the VMBus driver, the coming net/hvsock driver
> > > can register 2 callbacks and can know when a new hvsock connection is
> > > offered by the host, and when a hvsock connection is being closed by the
> > > host.
> > >
> > This is an extremely terrible interface.
> >
> > It's an opaque hook that allows on registry, and it's solve purpose
> > is to allow a backdoor call into a foreign driver in another module.
> >
> > These are exactly the things we try to avoid.
> 
> Hi David,
> Thanks a lot for your reviewing and the suggestion!
> 
> > Why not create a real abstraction where clients register an object,
> > that can be contained as a sub-member inside of their own driver
> > private, that provides the callback registry mechanism.

Hi David,
Can you please have a look at my below questions?

I like your idea of a real abstraction. Your answer would definitely
help me to implement that correctly. 

> Please pardon me for my inexperience.
> Can you please be a bit more specific?
> I guess maybe you're referencing a common design pattern in the driver
> code, so an example in some existing driver would be the best. :-)
> 
> "clients register an object " --
> does the "clients" mean the hvsock driver?
> and the "object" means the 2 callbacks?
> 
> IMHO, here the vmbus driver has to synchronously pass the 2 events
> to the hvsock driver, so a "backdoor call into the hvsock driver" is
> inevitable anyway?
> 
> e.g., in the path vmbus_process_offer() -> hvsock_process_offer(), the
> return value of the latter is important to the former, because on error
> the former needs to clean up some internal states of the vmbus driver (that
> is, the "goto err_deq_chan").
> 
> 
> > That way you can register multiple clients, do things like allow
> > AF_PACKET capturing of vmbus traffic, etc.
> 
> I thought AF_PACKET can only capture IP packets	or Ethernet frames.
> Can it be used to capture AF_UNIX packet?
> If yes, I suppose we can consider making it work for AF_HYPERV too,
> if people ask for that.
> 
> -- Dexuan

Thanks,
-- Dexuan

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to process hvsock connection
@ 2015-08-06  4:53       ` Dexuan Cui
  0 siblings, 0 replies; 18+ messages in thread
From: Dexuan Cui @ 2015-08-06  4:53 UTC (permalink / raw)
  To: David Miller, KY Srinivasan
  Cc: olaf, gregkh, jasowang, driverdev-devel, linux-kernel, stephen,
	stefanha, netdev, apw, pebolle, dan.carpenter

> From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On Behalf
> Of Dexuan Cui
> Sent: Thursday, July 30, 2015 18:20
> To: David Miller <davem@davemloft.net>; KY Srinivasan <kys@microsoft.com>
> Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> stephen@networkplumber.org; stefanha@redhat.com; netdev@vger.kernel.org;
> apw@canonical.com; pebolle@tiscali.nl; dan.carpenter@oracle.com
> Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to
> process hvsock connection
> 
> > From: David Miller
> > Sent: Thursday, July 30, 2015 6:27
> >
> > From: Dexuan Cui
> > Date: Tue, 28 Jul 2015 05:35:11 -0700
> >
> > > With the 2 APIs supplied by the VMBus driver, the coming net/hvsock driver
> > > can register 2 callbacks and can know when a new hvsock connection is
> > > offered by the host, and when a hvsock connection is being closed by the
> > > host.
> > >
> > This is an extremely terrible interface.
> >
> > It's an opaque hook that allows on registry, and it's solve purpose
> > is to allow a backdoor call into a foreign driver in another module.
> >
> > These are exactly the things we try to avoid.
> 
> Hi David,
> Thanks a lot for your reviewing and the suggestion!
> 
> > Why not create a real abstraction where clients register an object,
> > that can be contained as a sub-member inside of their own driver
> > private, that provides the callback registry mechanism.

Hi David,
Can you please have a look at my below questions?

I like your idea of a real abstraction. Your answer would definitely
help me to implement that correctly. 

> Please pardon me for my inexperience.
> Can you please be a bit more specific?
> I guess maybe you're referencing a common design pattern in the driver
> code, so an example in some existing driver would be the best. :-)
> 
> "clients register an object " --
> does the "clients" mean the hvsock driver?
> and the "object" means the 2 callbacks?
> 
> IMHO, here the vmbus driver has to synchronously pass the 2 events
> to the hvsock driver, so a "backdoor call into the hvsock driver" is
> inevitable anyway?
> 
> e.g., in the path vmbus_process_offer() -> hvsock_process_offer(), the
> return value of the latter is important to the former, because on error
> the former needs to clean up some internal states of the vmbus driver (that
> is, the "goto err_deq_chan").
> 
> 
> > That way you can register multiple clients, do things like allow
> > AF_PACKET capturing of vmbus traffic, etc.
> 
> I thought AF_PACKET can only capture IP packets	or Ethernet frames.
> Can it be used to capture AF_UNIX packet?
> If yes, I suppose we can consider making it work for AF_HYPERV too,
> if people ask for that.
> 
> -- Dexuan

Thanks,
-- Dexuan
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to process hvsock connection
  2015-08-06  4:53       ` Dexuan Cui
@ 2015-08-06 18:28         ` KY Srinivasan
  -1 siblings, 0 replies; 18+ messages in thread
From: KY Srinivasan @ 2015-08-06 18:28 UTC (permalink / raw)
  To: Dexuan Cui, David Miller
  Cc: olaf, gregkh, jasowang, driverdev-devel, linux-kernel, stephen,
	stefanha, netdev, apw, pebolle, dan.carpenter



> -----Original Message-----
> From: Dexuan Cui
> Sent: Wednesday, August 5, 2015 9:54 PM
> To: David Miller <davem@davemloft.net>; KY Srinivasan
> <kys@microsoft.com>
> Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> stephen@networkplumber.org; stefanha@redhat.com;
> netdev@vger.kernel.org; apw@canonical.com; pebolle@tiscali.nl;
> dan.carpenter@oracle.com
> Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks
> to process hvsock connection
> 
> > From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On
> Behalf
> > Of Dexuan Cui
> > Sent: Thursday, July 30, 2015 18:20
> > To: David Miller <davem@davemloft.net>; KY Srinivasan
> <kys@microsoft.com>
> > Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> > driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> > stephen@networkplumber.org; stefanha@redhat.com;
> netdev@vger.kernel.org;
> > apw@canonical.com; pebolle@tiscali.nl; dan.carpenter@oracle.com
> > Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register
> callbacks to
> > process hvsock connection
> >
> > > From: David Miller
> > > Sent: Thursday, July 30, 2015 6:27
> > >
> > > From: Dexuan Cui
> > > Date: Tue, 28 Jul 2015 05:35:11 -0700
> > >
> > > > With the 2 APIs supplied by the VMBus driver, the coming net/hvsock
> driver
> > > > can register 2 callbacks and can know when a new hvsock connection is
> > > > offered by the host, and when a hvsock connection is being closed by
> the
> > > > host.
> > > >
> > > This is an extremely terrible interface.
> > >
> > > It's an opaque hook that allows on registry, and it's solve purpose
> > > is to allow a backdoor call into a foreign driver in another module.
> > >
> > > These are exactly the things we try to avoid.
> >
> > Hi David,
> > Thanks a lot for your reviewing and the suggestion!
> >
> > > Why not create a real abstraction where clients register an object,
> > > that can be contained as a sub-member inside of their own driver
> > > private, that provides the callback registry mechanism.
> 
> Hi David,
> Can you please have a look at my below questions?
> 
> I like your idea of a real abstraction. Your answer would definitely
> help me to implement that correctly.
> 
> > Please pardon me for my inexperience.
> > Can you please be a bit more specific?
> > I guess maybe you're referencing a common design pattern in the driver
> > code, so an example in some existing driver would be the best. :-)
> >
> > "clients register an object " --
> > does the "clients" mean the hvsock driver?
> > and the "object" means the 2 callbacks?
> >
> > IMHO, here the vmbus driver has to synchronously pass the 2 events
> > to the hvsock driver, so a "backdoor call into the hvsock driver" is
> > inevitable anyway?
> >
> > e.g., in the path vmbus_process_offer() -> hvsock_process_offer(), the
> > return value of the latter is important to the former, because on error
> > the former needs to clean up some internal states of the vmbus driver
> (that
> > is, the "goto err_deq_chan").
> >
> >
> > > That way you can register multiple clients, do things like allow
> > > AF_PACKET capturing of vmbus traffic, etc.
> >
> > I thought AF_PACKET can only capture IP packetsor Ethernet frames.
> > Can it be used to capture AF_UNIX packet?
> > If yes, I suppose we can consider making it work for AF_HYPERV too,
> > if people ask for that.
> >

Dexuan,

The notion of a channel on Hyper-V has been mapped to a device on Linux and the mechanism we have
had of notifying the driver of the creation of the channel was through registering this device with the kernel
(vmbus_device_create). The first exception to this was when we introduced multi-channel support that broke
the assumption of this one to one mapping between the channel and Linux device. In the case of the sub-channels,
we handled the  driver notification issue via the sub-channel callback that the driver registers at the point of 
opening the channel. Perhaps we could make the sub-channel handling mechanism more generic to handle the case
of VMSOCK as well?

Regards,

K. Y
> > -- Dexuan
> 
> Thanks,
> -- Dexuan


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to process hvsock connection
@ 2015-08-06 18:28         ` KY Srinivasan
  0 siblings, 0 replies; 18+ messages in thread
From: KY Srinivasan @ 2015-08-06 18:28 UTC (permalink / raw)
  To: Dexuan Cui, David Miller
  Cc: olaf, gregkh, jasowang, driverdev-devel, linux-kernel, stephen,
	stefanha, netdev, apw, pebolle, dan.carpenter



> -----Original Message-----
> From: Dexuan Cui
> Sent: Wednesday, August 5, 2015 9:54 PM
> To: David Miller <davem@davemloft.net>; KY Srinivasan
> <kys@microsoft.com>
> Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> stephen@networkplumber.org; stefanha@redhat.com;
> netdev@vger.kernel.org; apw@canonical.com; pebolle@tiscali.nl;
> dan.carpenter@oracle.com
> Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks
> to process hvsock connection
> 
> > From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On
> Behalf
> > Of Dexuan Cui
> > Sent: Thursday, July 30, 2015 18:20
> > To: David Miller <davem@davemloft.net>; KY Srinivasan
> <kys@microsoft.com>
> > Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> > driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> > stephen@networkplumber.org; stefanha@redhat.com;
> netdev@vger.kernel.org;
> > apw@canonical.com; pebolle@tiscali.nl; dan.carpenter@oracle.com
> > Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register
> callbacks to
> > process hvsock connection
> >
> > > From: David Miller
> > > Sent: Thursday, July 30, 2015 6:27
> > >
> > > From: Dexuan Cui
> > > Date: Tue, 28 Jul 2015 05:35:11 -0700
> > >
> > > > With the 2 APIs supplied by the VMBus driver, the coming net/hvsock
> driver
> > > > can register 2 callbacks and can know when a new hvsock connection is
> > > > offered by the host, and when a hvsock connection is being closed by
> the
> > > > host.
> > > >
> > > This is an extremely terrible interface.
> > >
> > > It's an opaque hook that allows on registry, and it's solve purpose
> > > is to allow a backdoor call into a foreign driver in another module.
> > >
> > > These are exactly the things we try to avoid.
> >
> > Hi David,
> > Thanks a lot for your reviewing and the suggestion!
> >
> > > Why not create a real abstraction where clients register an object,
> > > that can be contained as a sub-member inside of their own driver
> > > private, that provides the callback registry mechanism.
> 
> Hi David,
> Can you please have a look at my below questions?
> 
> I like your idea of a real abstraction. Your answer would definitely
> help me to implement that correctly.
> 
> > Please pardon me for my inexperience.
> > Can you please be a bit more specific?
> > I guess maybe you're referencing a common design pattern in the driver
> > code, so an example in some existing driver would be the best. :-)
> >
> > "clients register an object " --
> > does the "clients" mean the hvsock driver?
> > and the "object" means the 2 callbacks?
> >
> > IMHO, here the vmbus driver has to synchronously pass the 2 events
> > to the hvsock driver, so a "backdoor call into the hvsock driver" is
> > inevitable anyway?
> >
> > e.g., in the path vmbus_process_offer() -> hvsock_process_offer(), the
> > return value of the latter is important to the former, because on error
> > the former needs to clean up some internal states of the vmbus driver
> (that
> > is, the "goto err_deq_chan").
> >
> >
> > > That way you can register multiple clients, do things like allow
> > > AF_PACKET capturing of vmbus traffic, etc.
> >
> > I thought AF_PACKET can only capture IP packetsor Ethernet frames.
> > Can it be used to capture AF_UNIX packet?
> > If yes, I suppose we can consider making it work for AF_HYPERV too,
> > if people ask for that.
> >

Dexuan,

The notion of a channel on Hyper-V has been mapped to a device on Linux and the mechanism we have
had of notifying the driver of the creation of the channel was through registering this device with the kernel
(vmbus_device_create). The first exception to this was when we introduced multi-channel support that broke
the assumption of this one to one mapping between the channel and Linux device. In the case of the sub-channels,
we handled the  driver notification issue via the sub-channel callback that the driver registers at the point of 
opening the channel. Perhaps we could make the sub-channel handling mechanism more generic to handle the case
of VMSOCK as well?

Regards,

K. Y
> > -- Dexuan
> 
> Thanks,
> -- Dexuan

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to process hvsock connection
  2015-08-06 18:28         ` KY Srinivasan
  (?)
@ 2015-08-07 10:26           ` Dexuan Cui
  -1 siblings, 0 replies; 18+ messages in thread
From: Dexuan Cui @ 2015-08-07 10:26 UTC (permalink / raw)
  To: KY Srinivasan, David Miller
  Cc: olaf, gregkh, jasowang, driverdev-devel, linux-kernel, stephen,
	stefanha, netdev, apw, pebolle, dan.carpenter

> -----Original Message-----
> From: KY Srinivasan
> Sent: Friday, August 7, 2015 2:28
> To: Dexuan Cui <decui@microsoft.com>; David Miller <davem@davemloft.net>
> Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> stephen@networkplumber.org; stefanha@redhat.com; netdev@vger.kernel.org;
> apw@canonical.com; pebolle@tiscali.nl; dan.carpenter@oracle.com
> Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to
> process hvsock connection
> 
> > -----Original Message-----
> > From: Dexuan Cui
> > Sent: Wednesday, August 5, 2015 9:54 PM
> > To: David Miller <davem@davemloft.net>; KY Srinivasan
> > <kys@microsoft.com>
> > Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> > driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> > stephen@networkplumber.org; stefanha@redhat.com;
> > netdev@vger.kernel.org; apw@canonical.com; pebolle@tiscali.nl;
> > dan.carpenter@oracle.com
> > Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks
> > to process hvsock connection
> >
> > > From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On
> > Behalf
> > > Of Dexuan Cui
> > > Sent: Thursday, July 30, 2015 18:20
> > > To: David Miller <davem@davemloft.net>; KY Srinivasan
> > <kys@microsoft.com>
> > > Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> > > driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> > > stephen@networkplumber.org; stefanha@redhat.com;
> > netdev@vger.kernel.org;
> > > apw@canonical.com; pebolle@tiscali.nl; dan.carpenter@oracle.com
> > > Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register
> > callbacks to
> > > process hvsock connection
> > >
> > > > From: David Miller
> > > > Sent: Thursday, July 30, 2015 6:27
> > > >
> > > > From: Dexuan Cui
> > > > Date: Tue, 28 Jul 2015 05:35:11 -0700
> > > >
> > > > > With the 2 APIs supplied by the VMBus driver, the coming net/hvsock
> > driver
> > > > > can register 2 callbacks and can know when a new hvsock connection is
> > > > > offered by the host, and when a hvsock connection is being closed by
> > the
> > > > > host.
> > > > >
> > > > This is an extremely terrible interface.
> > > >
> > > > It's an opaque hook that allows on registry, and it's solve purpose
> > > > is to allow a backdoor call into a foreign driver in another module.
> > > >
> > > > These are exactly the things we try to avoid.
> > >
> > > Hi David,
> > > Thanks a lot for your reviewing and the suggestion!
> > >
> > > > Why not create a real abstraction where clients register an object,
> > > > that can be contained as a sub-member inside of their own driver
> > > > private, that provides the callback registry mechanism.
> >
> > Hi David,
> > Can you please have a look at my below questions?
> >
> > I like your idea of a real abstraction. Your answer would definitely
> > help me to implement that correctly.
> >
> > > Please pardon me for my inexperience.
> > > Can you please be a bit more specific?
> > > I guess maybe you're referencing a common design pattern in the driver
> > > code, so an example in some existing driver would be the best. :-)
> > >
> > > "clients register an object " --
> > > does the "clients" mean the hvsock driver?
> > > and the "object" means the 2 callbacks?
> > >
> > > IMHO, here the vmbus driver has to synchronously pass the 2 events
> > > to the hvsock driver, so a "backdoor call into the hvsock driver" is
> > > inevitable anyway?
> > >
> > > e.g., in the path vmbus_process_offer() -> hvsock_process_offer(), the
> > > return value of the latter is important to the former, because on error
> > > the former needs to clean up some internal states of the vmbus driver
> > (that
> > > is, the "goto err_deq_chan").
> > >
> > >
> > > > That way you can register multiple clients, do things like allow
> > > > AF_PACKET capturing of vmbus traffic, etc.
> > >
> > > I thought AF_PACKET can only capture IP packetsor Ethernet frames.
> > > Can it be used to capture AF_UNIX packet?
> > > If yes, I suppose we can consider making it work for AF_HYPERV too,
> > > if people ask for that.
> > >
> 
> Dexuan,
> 
> The notion of a channel on Hyper-V has been mapped to a device on Linux and
> the mechanism we have
> had of notifying the driver of the creation of the channel was through
> registering this device with the kernel
> (vmbus_device_create). The first exception to this was when we introduced
> multi-channel support that broke
> the assumption of this one to one mapping between the channel and Linux
> device. In the case of the sub-channels,
> we handled the  driver notification issue via the sub-channel callback that the
> driver registers at the point of
> opening the channel. Perhaps we could make the sub-channel handling
> mechanism more generic to handle the case
> of VMSOCK as well?
> 
> K. Y

Good suggestion!
Let me think this over and make a new patch.

Thanks,
-- Dexuan


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to process hvsock connection
@ 2015-08-07 10:26           ` Dexuan Cui
  0 siblings, 0 replies; 18+ messages in thread
From: Dexuan Cui @ 2015-08-07 10:26 UTC (permalink / raw)
  To: KY Srinivasan, David Miller
  Cc: olaf, gregkh, jasowang, driverdev-devel, linux-kernel, stephen,
	stefanha, netdev, apw, pebolle, dan.carpenter

> -----Original Message-----
> From: KY Srinivasan
> Sent: Friday, August 7, 2015 2:28
> To: Dexuan Cui <decui@microsoft.com>; David Miller <davem@davemloft.net>
> Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> stephen@networkplumber.org; stefanha@redhat.com; netdev@vger.kernel.org;
> apw@canonical.com; pebolle@tiscali.nl; dan.carpenter@oracle.com
> Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to
> process hvsock connection
> 
> > -----Original Message-----
> > From: Dexuan Cui
> > Sent: Wednesday, August 5, 2015 9:54 PM
> > To: David Miller <davem@davemloft.net>; KY Srinivasan
> > <kys@microsoft.com>
> > Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> > driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> > stephen@networkplumber.org; stefanha@redhat.com;
> > netdev@vger.kernel.org; apw@canonical.com; pebolle@tiscali.nl;
> > dan.carpenter@oracle.com
> > Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks
> > to process hvsock connection
> >
> > > From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On
> > Behalf
> > > Of Dexuan Cui
> > > Sent: Thursday, July 30, 2015 18:20
> > > To: David Miller <davem@davemloft.net>; KY Srinivasan
> > <kys@microsoft.com>
> > > Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> > > driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> > > stephen@networkplumber.org; stefanha@redhat.com;
> > netdev@vger.kernel.org;
> > > apw@canonical.com; pebolle@tiscali.nl; dan.carpenter@oracle.com
> > > Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register
> > callbacks to
> > > process hvsock connection
> > >
> > > > From: David Miller
> > > > Sent: Thursday, July 30, 2015 6:27
> > > >
> > > > From: Dexuan Cui
> > > > Date: Tue, 28 Jul 2015 05:35:11 -0700
> > > >
> > > > > With the 2 APIs supplied by the VMBus driver, the coming net/hvsock
> > driver
> > > > > can register 2 callbacks and can know when a new hvsock connection is
> > > > > offered by the host, and when a hvsock connection is being closed by
> > the
> > > > > host.
> > > > >
> > > > This is an extremely terrible interface.
> > > >
> > > > It's an opaque hook that allows on registry, and it's solve purpose
> > > > is to allow a backdoor call into a foreign driver in another module.
> > > >
> > > > These are exactly the things we try to avoid.
> > >
> > > Hi David,
> > > Thanks a lot for your reviewing and the suggestion!
> > >
> > > > Why not create a real abstraction where clients register an object,
> > > > that can be contained as a sub-member inside of their own driver
> > > > private, that provides the callback registry mechanism.
> >
> > Hi David,
> > Can you please have a look at my below questions?
> >
> > I like your idea of a real abstraction. Your answer would definitely
> > help me to implement that correctly.
> >
> > > Please pardon me for my inexperience.
> > > Can you please be a bit more specific?
> > > I guess maybe you're referencing a common design pattern in the driver
> > > code, so an example in some existing driver would be the best. :-)
> > >
> > > "clients register an object " --
> > > does the "clients" mean the hvsock driver?
> > > and the "object" means the 2 callbacks?
> > >
> > > IMHO, here the vmbus driver has to synchronously pass the 2 events
> > > to the hvsock driver, so a "backdoor call into the hvsock driver" is
> > > inevitable anyway?
> > >
> > > e.g., in the path vmbus_process_offer() -> hvsock_process_offer(), the
> > > return value of the latter is important to the former, because on error
> > > the former needs to clean up some internal states of the vmbus driver
> > (that
> > > is, the "goto err_deq_chan").
> > >
> > >
> > > > That way you can register multiple clients, do things like allow
> > > > AF_PACKET capturing of vmbus traffic, etc.
> > >
> > > I thought AF_PACKET can only capture IP packetsor Ethernet frames.
> > > Can it be used to capture AF_UNIX packet?
> > > If yes, I suppose we can consider making it work for AF_HYPERV too,
> > > if people ask for that.
> > >
> 
> Dexuan,
> 
> The notion of a channel on Hyper-V has been mapped to a device on Linux and
> the mechanism we have
> had of notifying the driver of the creation of the channel was through
> registering this device with the kernel
> (vmbus_device_create). The first exception to this was when we introduced
> multi-channel support that broke
> the assumption of this one to one mapping between the channel and Linux
> device. In the case of the sub-channels,
> we handled the  driver notification issue via the sub-channel callback that the
> driver registers at the point of
> opening the channel. Perhaps we could make the sub-channel handling
> mechanism more generic to handle the case
> of VMSOCK as well?
> 
> K. Y

Good suggestion!
Let me think this over and make a new patch.

Thanks,
-- Dexuan

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to process hvsock connection
@ 2015-08-07 10:26           ` Dexuan Cui
  0 siblings, 0 replies; 18+ messages in thread
From: Dexuan Cui @ 2015-08-07 10:26 UTC (permalink / raw)
  To: KY Srinivasan, David Miller
  Cc: olaf, gregkh, jasowang, driverdev-devel, linux-kernel, stephen,
	stefanha, netdev, apw, pebolle, dan.carpenter

> -----Original Message-----
> From: KY Srinivasan
> Sent: Friday, August 7, 2015 2:28
> To: Dexuan Cui <decui@microsoft.com>; David Miller <davem@davemloft.net>
> Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> stephen@networkplumber.org; stefanha@redhat.com; netdev@vger.kernel.org;
> apw@canonical.com; pebolle@tiscali.nl; dan.carpenter@oracle.com
> Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to
> process hvsock connection
> 
> > -----Original Message-----
> > From: Dexuan Cui
> > Sent: Wednesday, August 5, 2015 9:54 PM
> > To: David Miller <davem@davemloft.net>; KY Srinivasan
> > <kys@microsoft.com>
> > Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> > driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> > stephen@networkplumber.org; stefanha@redhat.com;
> > netdev@vger.kernel.org; apw@canonical.com; pebolle@tiscali.nl;
> > dan.carpenter@oracle.com
> > Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks
> > to process hvsock connection
> >
> > > From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On
> > Behalf
> > > Of Dexuan Cui
> > > Sent: Thursday, July 30, 2015 18:20
> > > To: David Miller <davem@davemloft.net>; KY Srinivasan
> > <kys@microsoft.com>
> > > Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> > > driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> > > stephen@networkplumber.org; stefanha@redhat.com;
> > netdev@vger.kernel.org;
> > > apw@canonical.com; pebolle@tiscali.nl; dan.carpenter@oracle.com
> > > Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register
> > callbacks to
> > > process hvsock connection
> > >
> > > > From: David Miller
> > > > Sent: Thursday, July 30, 2015 6:27
> > > >
> > > > From: Dexuan Cui
> > > > Date: Tue, 28 Jul 2015 05:35:11 -0700
> > > >
> > > > > With the 2 APIs supplied by the VMBus driver, the coming net/hvsock
> > driver
> > > > > can register 2 callbacks and can know when a new hvsock connection is
> > > > > offered by the host, and when a hvsock connection is being closed by
> > the
> > > > > host.
> > > > >
> > > > This is an extremely terrible interface.
> > > >
> > > > It's an opaque hook that allows on registry, and it's solve purpose
> > > > is to allow a backdoor call into a foreign driver in another module.
> > > >
> > > > These are exactly the things we try to avoid.
> > >
> > > Hi David,
> > > Thanks a lot for your reviewing and the suggestion!
> > >
> > > > Why not create a real abstraction where clients register an object,
> > > > that can be contained as a sub-member inside of their own driver
> > > > private, that provides the callback registry mechanism.
> >
> > Hi David,
> > Can you please have a look at my below questions?
> >
> > I like your idea of a real abstraction. Your answer would definitely
> > help me to implement that correctly.
> >
> > > Please pardon me for my inexperience.
> > > Can you please be a bit more specific?
> > > I guess maybe you're referencing a common design pattern in the driver
> > > code, so an example in some existing driver would be the best. :-)
> > >
> > > "clients register an object " --
> > > does the "clients" mean the hvsock driver?
> > > and the "object" means the 2 callbacks?
> > >
> > > IMHO, here the vmbus driver has to synchronously pass the 2 events
> > > to the hvsock driver, so a "backdoor call into the hvsock driver" is
> > > inevitable anyway?
> > >
> > > e.g., in the path vmbus_process_offer() -> hvsock_process_offer(), the
> > > return value of the latter is important to the former, because on error
> > > the former needs to clean up some internal states of the vmbus driver
> > (that
> > > is, the "goto err_deq_chan").
> > >
> > >
> > > > That way you can register multiple clients, do things like allow
> > > > AF_PACKET capturing of vmbus traffic, etc.
> > >
> > > I thought AF_PACKET can only capture IP packetsor Ethernet frames.
> > > Can it be used to capture AF_UNIX packet?
> > > If yes, I suppose we can consider making it work for AF_HYPERV too,
> > > if people ask for that.
> > >
> 
> Dexuan,
> 
> The notion of a channel on Hyper-V has been mapped to a device on Linux and
> the mechanism we have
> had of notifying the driver of the creation of the channel was through
> registering this device with the kernel
> (vmbus_device_create). The first exception to this was when we introduced
> multi-channel support that broke
> the assumption of this one to one mapping between the channel and Linux
> device. In the case of the sub-channels,
> we handled the  driver notification issue via the sub-channel callback that the
> driver registers at the point of
> opening the channel. Perhaps we could make the sub-channel handling
> mechanism more generic to handle the case
> of VMSOCK as well?
> 
> K. Y

Good suggestion!
Let me think this over and make a new patch.

Thanks,
-- Dexuan

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to process hvsock connection
  2015-08-07 10:26           ` Dexuan Cui
  (?)
@ 2015-08-18  5:07             ` KY Srinivasan
  -1 siblings, 0 replies; 18+ messages in thread
From: KY Srinivasan @ 2015-08-18  5:07 UTC (permalink / raw)
  To: Dexuan Cui, David Miller
  Cc: olaf, gregkh, jasowang, driverdev-devel, linux-kernel, stephen,
	stefanha, netdev, apw, pebolle, dan.carpenter



> -----Original Message-----
> From: Dexuan Cui
> Sent: Friday, August 7, 2015 3:27 AM
> To: KY Srinivasan <kys@microsoft.com>; David Miller
> <davem@davemloft.net>
> Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> stephen@networkplumber.org; stefanha@redhat.com;
> netdev@vger.kernel.org; apw@canonical.com; pebolle@tiscali.nl;
> dan.carpenter@oracle.com
> Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks
> to process hvsock connection
> 
> > -----Original Message-----
> > From: KY Srinivasan
> > Sent: Friday, August 7, 2015 2:28
> > To: Dexuan Cui <decui@microsoft.com>; David Miller
> <davem@davemloft.net>
> > Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> > driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> > stephen@networkplumber.org; stefanha@redhat.com;
> netdev@vger.kernel.org;
> > apw@canonical.com; pebolle@tiscali.nl; dan.carpenter@oracle.com
> > Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register
> callbacks to
> > process hvsock connection
> >
> > > -----Original Message-----
> > > From: Dexuan Cui
> > > Sent: Wednesday, August 5, 2015 9:54 PM
> > > To: David Miller <davem@davemloft.net>; KY Srinivasan
> > > <kys@microsoft.com>
> > > Cc: olaf@aepfle.de; gregkh@linuxfoundation.org;
> jasowang@redhat.com;
> > > driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> > > stephen@networkplumber.org; stefanha@redhat.com;
> > > netdev@vger.kernel.org; apw@canonical.com; pebolle@tiscali.nl;
> > > dan.carpenter@oracle.com
> > > Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register
> callbacks
> > > to process hvsock connection
> > >
> > > > From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org]
> On
> > > Behalf
> > > > Of Dexuan Cui
> > > > Sent: Thursday, July 30, 2015 18:20
> > > > To: David Miller <davem@davemloft.net>; KY Srinivasan
> > > <kys@microsoft.com>
> > > > Cc: olaf@aepfle.de; gregkh@linuxfoundation.org;
> jasowang@redhat.com;
> > > > driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> > > > stephen@networkplumber.org; stefanha@redhat.com;
> > > netdev@vger.kernel.org;
> > > > apw@canonical.com; pebolle@tiscali.nl; dan.carpenter@oracle.com
> > > > Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register
> > > callbacks to
> > > > process hvsock connection
> > > >
> > > > > From: David Miller
> > > > > Sent: Thursday, July 30, 2015 6:27
> > > > >
> > > > > From: Dexuan Cui
> > > > > Date: Tue, 28 Jul 2015 05:35:11 -0700
> > > > >
> > > > > > With the 2 APIs supplied by the VMBus driver, the coming
> net/hvsock
> > > driver
> > > > > > can register 2 callbacks and can know when a new hvsock
> connection is
> > > > > > offered by the host, and when a hvsock connection is being closed
> by
> > > the
> > > > > > host.
> > > > > >
> > > > > This is an extremely terrible interface.
> > > > >
> > > > > It's an opaque hook that allows on registry, and it's solve purpose
> > > > > is to allow a backdoor call into a foreign driver in another module.
> > > > >
> > > > > These are exactly the things we try to avoid.
> > > >
> > > > Hi David,
> > > > Thanks a lot for your reviewing and the suggestion!
> > > >
> > > > > Why not create a real abstraction where clients register an object,
> > > > > that can be contained as a sub-member inside of their own driver
> > > > > private, that provides the callback registry mechanism.
> > >
> > > Hi David,
> > > Can you please have a look at my below questions?
> > >
> > > I like your idea of a real abstraction. Your answer would definitely
> > > help me to implement that correctly.
> > >
> > > > Please pardon me for my inexperience.
> > > > Can you please be a bit more specific?
> > > > I guess maybe you're referencing a common design pattern in the
> driver
> > > > code, so an example in some existing driver would be the best. :-)
> > > >
> > > > "clients register an object " --
> > > > does the "clients" mean the hvsock driver?
> > > > and the "object" means the 2 callbacks?
> > > >
> > > > IMHO, here the vmbus driver has to synchronously pass the 2 events
> > > > to the hvsock driver, so a "backdoor call into the hvsock driver" is
> > > > inevitable anyway?
> > > >
> > > > e.g., in the path vmbus_process_offer() -> hvsock_process_offer(), the
> > > > return value of the latter is important to the former, because on error
> > > > the former needs to clean up some internal states of the vmbus driver
> > > (that
> > > > is, the "goto err_deq_chan").
> > > >
> > > >
> > > > > That way you can register multiple clients, do things like allow
> > > > > AF_PACKET capturing of vmbus traffic, etc.
> > > >
> > > > I thought AF_PACKET can only capture IP packetsor Ethernet frames.
> > > > Can it be used to capture AF_UNIX packet?
> > > > If yes, I suppose we can consider making it work for AF_HYPERV too,
> > > > if people ask for that.
> > > >
> >
> > Dexuan,
> >
> > The notion of a channel on Hyper-V has been mapped to a device on Linux
> and
> > the mechanism we have
> > had of notifying the driver of the creation of the channel was through
> > registering this device with the kernel
> > (vmbus_device_create). The first exception to this was when we
> introduced
> > multi-channel support that broke
> > the assumption of this one to one mapping between the channel and Linux
> > device. In the case of the sub-channels,
> > we handled the  driver notification issue via the sub-channel callback that
> the
> > driver registers at the point of
> > opening the channel. Perhaps we could make the sub-channel handling
> > mechanism more generic to handle the case
> > of VMSOCK as well?
> >
> > K. Y
> 
> Good suggestion!
> Let me think this over and make a new patch.
> 
> Thanks,
> -- Dexuan
> 
Looks like VMSOCK handling cannot be accommodated as an extension of sub-channel handling.
However, I still think we should use the standard driver core primitives to handle VMSOCK devices.
What if we extend the vmbus match function to accommodate vmsock devices. Here is one way we
could extend the vmbus match function:

if (the device is not VMSOCK) {
	Match the device type ID with the ID table supported by the driver
} else {
	Match is based on a special attribute of the driver
}

By doing so, we can use the standard probe call to open the vmsock channel and use the standard mechanism for removing the device.

Regards,

K. Y

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to process hvsock connection
@ 2015-08-18  5:07             ` KY Srinivasan
  0 siblings, 0 replies; 18+ messages in thread
From: KY Srinivasan @ 2015-08-18  5:07 UTC (permalink / raw)
  To: Dexuan Cui, David Miller
  Cc: olaf, gregkh, jasowang, driverdev-devel, linux-kernel, stephen,
	stefanha, netdev, apw, pebolle, dan.carpenter



> -----Original Message-----
> From: Dexuan Cui
> Sent: Friday, August 7, 2015 3:27 AM
> To: KY Srinivasan <kys@microsoft.com>; David Miller
> <davem@davemloft.net>
> Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> stephen@networkplumber.org; stefanha@redhat.com;
> netdev@vger.kernel.org; apw@canonical.com; pebolle@tiscali.nl;
> dan.carpenter@oracle.com
> Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks
> to process hvsock connection
> 
> > -----Original Message-----
> > From: KY Srinivasan
> > Sent: Friday, August 7, 2015 2:28
> > To: Dexuan Cui <decui@microsoft.com>; David Miller
> <davem@davemloft.net>
> > Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> > driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> > stephen@networkplumber.org; stefanha@redhat.com;
> netdev@vger.kernel.org;
> > apw@canonical.com; pebolle@tiscali.nl; dan.carpenter@oracle.com
> > Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register
> callbacks to
> > process hvsock connection
> >
> > > -----Original Message-----
> > > From: Dexuan Cui
> > > Sent: Wednesday, August 5, 2015 9:54 PM
> > > To: David Miller <davem@davemloft.net>; KY Srinivasan
> > > <kys@microsoft.com>
> > > Cc: olaf@aepfle.de; gregkh@linuxfoundation.org;
> jasowang@redhat.com;
> > > driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> > > stephen@networkplumber.org; stefanha@redhat.com;
> > > netdev@vger.kernel.org; apw@canonical.com; pebolle@tiscali.nl;
> > > dan.carpenter@oracle.com
> > > Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register
> callbacks
> > > to process hvsock connection
> > >
> > > > From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org]
> On
> > > Behalf
> > > > Of Dexuan Cui
> > > > Sent: Thursday, July 30, 2015 18:20
> > > > To: David Miller <davem@davemloft.net>; KY Srinivasan
> > > <kys@microsoft.com>
> > > > Cc: olaf@aepfle.de; gregkh@linuxfoundation.org;
> jasowang@redhat.com;
> > > > driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> > > > stephen@networkplumber.org; stefanha@redhat.com;
> > > netdev@vger.kernel.org;
> > > > apw@canonical.com; pebolle@tiscali.nl; dan.carpenter@oracle.com
> > > > Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register
> > > callbacks to
> > > > process hvsock connection
> > > >
> > > > > From: David Miller
> > > > > Sent: Thursday, July 30, 2015 6:27
> > > > >
> > > > > From: Dexuan Cui
> > > > > Date: Tue, 28 Jul 2015 05:35:11 -0700
> > > > >
> > > > > > With the 2 APIs supplied by the VMBus driver, the coming
> net/hvsock
> > > driver
> > > > > > can register 2 callbacks and can know when a new hvsock
> connection is
> > > > > > offered by the host, and when a hvsock connection is being closed
> by
> > > the
> > > > > > host.
> > > > > >
> > > > > This is an extremely terrible interface.
> > > > >
> > > > > It's an opaque hook that allows on registry, and it's solve purpose
> > > > > is to allow a backdoor call into a foreign driver in another module.
> > > > >
> > > > > These are exactly the things we try to avoid.
> > > >
> > > > Hi David,
> > > > Thanks a lot for your reviewing and the suggestion!
> > > >
> > > > > Why not create a real abstraction where clients register an object,
> > > > > that can be contained as a sub-member inside of their own driver
> > > > > private, that provides the callback registry mechanism.
> > >
> > > Hi David,
> > > Can you please have a look at my below questions?
> > >
> > > I like your idea of a real abstraction. Your answer would definitely
> > > help me to implement that correctly.
> > >
> > > > Please pardon me for my inexperience.
> > > > Can you please be a bit more specific?
> > > > I guess maybe you're referencing a common design pattern in the
> driver
> > > > code, so an example in some existing driver would be the best. :-)
> > > >
> > > > "clients register an object " --
> > > > does the "clients" mean the hvsock driver?
> > > > and the "object" means the 2 callbacks?
> > > >
> > > > IMHO, here the vmbus driver has to synchronously pass the 2 events
> > > > to the hvsock driver, so a "backdoor call into the hvsock driver" is
> > > > inevitable anyway?
> > > >
> > > > e.g., in the path vmbus_process_offer() -> hvsock_process_offer(), the
> > > > return value of the latter is important to the former, because on error
> > > > the former needs to clean up some internal states of the vmbus driver
> > > (that
> > > > is, the "goto err_deq_chan").
> > > >
> > > >
> > > > > That way you can register multiple clients, do things like allow
> > > > > AF_PACKET capturing of vmbus traffic, etc.
> > > >
> > > > I thought AF_PACKET can only capture IP packetsor Ethernet frames.
> > > > Can it be used to capture AF_UNIX packet?
> > > > If yes, I suppose we can consider making it work for AF_HYPERV too,
> > > > if people ask for that.
> > > >
> >
> > Dexuan,
> >
> > The notion of a channel on Hyper-V has been mapped to a device on Linux
> and
> > the mechanism we have
> > had of notifying the driver of the creation of the channel was through
> > registering this device with the kernel
> > (vmbus_device_create). The first exception to this was when we
> introduced
> > multi-channel support that broke
> > the assumption of this one to one mapping between the channel and Linux
> > device. In the case of the sub-channels,
> > we handled the  driver notification issue via the sub-channel callback that
> the
> > driver registers at the point of
> > opening the channel. Perhaps we could make the sub-channel handling
> > mechanism more generic to handle the case
> > of VMSOCK as well?
> >
> > K. Y
> 
> Good suggestion!
> Let me think this over and make a new patch.
> 
> Thanks,
> -- Dexuan
> 
Looks like VMSOCK handling cannot be accommodated as an extension of sub-channel handling.
However, I still think we should use the standard driver core primitives to handle VMSOCK devices.
What if we extend the vmbus match function to accommodate vmsock devices. Here is one way we
could extend the vmbus match function:

if (the device is not VMSOCK) {
	Match the device type ID with the ID table supported by the driver
} else {
	Match is based on a special attribute of the driver
}

By doing so, we can use the standard probe call to open the vmsock channel and use the standard mechanism for removing the device.

Regards,

K. Y

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to process hvsock connection
@ 2015-08-18  5:07             ` KY Srinivasan
  0 siblings, 0 replies; 18+ messages in thread
From: KY Srinivasan @ 2015-08-18  5:07 UTC (permalink / raw)
  To: Dexuan Cui, David Miller
  Cc: olaf, gregkh, jasowang, driverdev-devel, linux-kernel, stephen,
	stefanha, netdev, apw, pebolle, dan.carpenter



> -----Original Message-----
> From: Dexuan Cui
> Sent: Friday, August 7, 2015 3:27 AM
> To: KY Srinivasan <kys@microsoft.com>; David Miller
> <davem@davemloft.net>
> Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> stephen@networkplumber.org; stefanha@redhat.com;
> netdev@vger.kernel.org; apw@canonical.com; pebolle@tiscali.nl;
> dan.carpenter@oracle.com
> Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks
> to process hvsock connection
> 
> > -----Original Message-----
> > From: KY Srinivasan
> > Sent: Friday, August 7, 2015 2:28
> > To: Dexuan Cui <decui@microsoft.com>; David Miller
> <davem@davemloft.net>
> > Cc: olaf@aepfle.de; gregkh@linuxfoundation.org; jasowang@redhat.com;
> > driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> > stephen@networkplumber.org; stefanha@redhat.com;
> netdev@vger.kernel.org;
> > apw@canonical.com; pebolle@tiscali.nl; dan.carpenter@oracle.com
> > Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register
> callbacks to
> > process hvsock connection
> >
> > > -----Original Message-----
> > > From: Dexuan Cui
> > > Sent: Wednesday, August 5, 2015 9:54 PM
> > > To: David Miller <davem@davemloft.net>; KY Srinivasan
> > > <kys@microsoft.com>
> > > Cc: olaf@aepfle.de; gregkh@linuxfoundation.org;
> jasowang@redhat.com;
> > > driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> > > stephen@networkplumber.org; stefanha@redhat.com;
> > > netdev@vger.kernel.org; apw@canonical.com; pebolle@tiscali.nl;
> > > dan.carpenter@oracle.com
> > > Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register
> callbacks
> > > to process hvsock connection
> > >
> > > > From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org]
> On
> > > Behalf
> > > > Of Dexuan Cui
> > > > Sent: Thursday, July 30, 2015 18:20
> > > > To: David Miller <davem@davemloft.net>; KY Srinivasan
> > > <kys@microsoft.com>
> > > > Cc: olaf@aepfle.de; gregkh@linuxfoundation.org;
> jasowang@redhat.com;
> > > > driverdev-devel@linuxdriverproject.org; linux-kernel@vger.kernel.org;
> > > > stephen@networkplumber.org; stefanha@redhat.com;
> > > netdev@vger.kernel.org;
> > > > apw@canonical.com; pebolle@tiscali.nl; dan.carpenter@oracle.com
> > > > Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register
> > > callbacks to
> > > > process hvsock connection
> > > >
> > > > > From: David Miller
> > > > > Sent: Thursday, July 30, 2015 6:27
> > > > >
> > > > > From: Dexuan Cui
> > > > > Date: Tue, 28 Jul 2015 05:35:11 -0700
> > > > >
> > > > > > With the 2 APIs supplied by the VMBus driver, the coming
> net/hvsock
> > > driver
> > > > > > can register 2 callbacks and can know when a new hvsock
> connection is
> > > > > > offered by the host, and when a hvsock connection is being closed
> by
> > > the
> > > > > > host.
> > > > > >
> > > > > This is an extremely terrible interface.
> > > > >
> > > > > It's an opaque hook that allows on registry, and it's solve purpose
> > > > > is to allow a backdoor call into a foreign driver in another module.
> > > > >
> > > > > These are exactly the things we try to avoid.
> > > >
> > > > Hi David,
> > > > Thanks a lot for your reviewing and the suggestion!
> > > >
> > > > > Why not create a real abstraction where clients register an object,
> > > > > that can be contained as a sub-member inside of their own driver
> > > > > private, that provides the callback registry mechanism.
> > >
> > > Hi David,
> > > Can you please have a look at my below questions?
> > >
> > > I like your idea of a real abstraction. Your answer would definitely
> > > help me to implement that correctly.
> > >
> > > > Please pardon me for my inexperience.
> > > > Can you please be a bit more specific?
> > > > I guess maybe you're referencing a common design pattern in the
> driver
> > > > code, so an example in some existing driver would be the best. :-)
> > > >
> > > > "clients register an object " --
> > > > does the "clients" mean the hvsock driver?
> > > > and the "object" means the 2 callbacks?
> > > >
> > > > IMHO, here the vmbus driver has to synchronously pass the 2 events
> > > > to the hvsock driver, so a "backdoor call into the hvsock driver" is
> > > > inevitable anyway?
> > > >
> > > > e.g., in the path vmbus_process_offer() -> hvsock_process_offer(), the
> > > > return value of the latter is important to the former, because on error
> > > > the former needs to clean up some internal states of the vmbus driver
> > > (that
> > > > is, the "goto err_deq_chan").
> > > >
> > > >
> > > > > That way you can register multiple clients, do things like allow
> > > > > AF_PACKET capturing of vmbus traffic, etc.
> > > >
> > > > I thought AF_PACKET can only capture IP packetsor Ethernet frames.
> > > > Can it be used to capture AF_UNIX packet?
> > > > If yes, I suppose we can consider making it work for AF_HYPERV too,
> > > > if people ask for that.
> > > >
> >
> > Dexuan,
> >
> > The notion of a channel on Hyper-V has been mapped to a device on Linux
> and
> > the mechanism we have
> > had of notifying the driver of the creation of the channel was through
> > registering this device with the kernel
> > (vmbus_device_create). The first exception to this was when we
> introduced
> > multi-channel support that broke
> > the assumption of this one to one mapping between the channel and Linux
> > device. In the case of the sub-channels,
> > we handled the  driver notification issue via the sub-channel callback that
> the
> > driver registers at the point of
> > opening the channel. Perhaps we could make the sub-channel handling
> > mechanism more generic to handle the case
> > of VMSOCK as well?
> >
> > K. Y
> 
> Good suggestion!
> Let me think this over and make a new patch.
> 
> Thanks,
> -- Dexuan
> 
Looks like VMSOCK handling cannot be accommodated as an extension of sub-channel handling.
However, I still think we should use the standard driver core primitives to handle VMSOCK devices.
What if we extend the vmbus match function to accommodate vmsock devices. Here is one way we
could extend the vmbus match function:

if (the device is not VMSOCK) {
	Match the device type ID with the ID table supported by the driver
} else {
	Match is based on a special attribute of the driver
}

By doing so, we can use the standard probe call to open the vmsock channel and use the standard mechanism for removing the device.

Regards,

K. Y
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2015-08-18  5:07 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-28 12:35 [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to process hvsock connection Dexuan Cui
2015-07-28 12:35 ` Dexuan Cui
2015-07-29 22:26 ` David Miller
2015-07-29 22:26   ` David Miller
2015-07-30 10:20   ` Dexuan Cui
2015-07-30 10:20     ` Dexuan Cui
2015-07-30 10:20     ` Dexuan Cui
2015-08-06  4:53     ` Dexuan Cui
2015-08-06  4:53       ` Dexuan Cui
2015-08-06  4:53       ` Dexuan Cui
2015-08-06 18:28       ` KY Srinivasan
2015-08-06 18:28         ` KY Srinivasan
2015-08-07 10:26         ` Dexuan Cui
2015-08-07 10:26           ` Dexuan Cui
2015-08-07 10:26           ` Dexuan Cui
2015-08-18  5:07           ` KY Srinivasan
2015-08-18  5:07             ` KY Srinivasan
2015-08-18  5:07             ` KY Srinivasan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.