From: Dexuan Cui <decui@microsoft.com>
To: KY Srinivasan <kys@microsoft.com>, David Miller <davem@davemloft.net>
CC: "olaf@aepfle.de" <olaf@aepfle.de>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "jasowang@redhat.com" <jasowang@redhat.com>,
        "driverdev-devel@linuxdriverproject.org" 
	<driverdev-devel@linuxdriverproject.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "stephen@networkplumber.org" <stephen@networkplumber.org>,
        "stefanha@redhat.com" <stefanha@redhat.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "apw@canonical.com" <apw@canonical.com>,
        "pebolle@tiscali.nl" <pebolle@tiscali.nl>,
        "dan.carpenter@oracle.com" <dan.carpenter@oracle.com>
Subject: RE: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register callbacks
 to process hvsock connection
Thread-Topic: [PATCH V4 4/7] Drivers: hv: vmbus: add APIs to register
 callbacks to process hvsock connection
Thread-Index: AQHQyk2vp4eFpUHUkUGmpaGSjS0WOJ3zwGiQgAqwL6CAAOYiAIABC0/Q
Date: Fri, 7 Aug 2015 10:26:57 +0000
Message-ID: <bf376c454de0483ba13c34db8418bb95@SIXPR30MB031.064d.mgd.msft.net>
References: <1438086911-19384-1-git-send-email-decui@microsoft.com>
 <20150729.152651.331017965932467028.davem@davemloft.net>
 <f65ab29ed3d0488685afdca2dd2189a9@SIXPR30MB031.064d.mgd.msft.net>
 <115a97288d1e4c239fcbaab28298c552@SIXPR30MB031.064d.mgd.msft.net>
 <SN1PR0301MB1662D4C6D4EA3F82FAA4C4C7A0740@SN1PR0301MB1662.namprd03.prod.outlook.com>
In-Reply-To: <SN1PR0301MB1662D4C6D4EA3F82FAA4C4C7A0740@SN1PR0301MB1662.namprd03.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Aug 2015 10:27:04.5795
 (UTC)
X-MS-Exchange-CrossTenant-Id: 72f988bf-86f1-41af-91ab-2d7cd011db47
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=72f988bf-86f1-41af-91ab-2d7cd011db47;Ip=[206.191.229.116];Helo=[064-smtp-out.microsoft.com]
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR0301MB1260
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 5292
Lines: 130

> -----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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/