RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks to process hvsock connection

[Dexuan Cui <decui@microsoft.com>]

> 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