Re: [PATCH RFC 0/3] Drivers: hv: utils: re-implement the kernel/userspace communication layer

[Vitaly Kuznetsov <vkuznets@redhat.com>]

KY Srinivasan <kys@microsoft.com> writes:

>> -----Original Message-----
>> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
>> Sent: Friday, February 27, 2015 8:14 AM
>> To: KY Srinivasan; devel@linuxdriverproject.org
>> Cc: Haiyang Zhang; linux-kernel@vger.kernel.org; Dexuan Cui; Radim Krčmář;
>> Greg Kroah-Hartman; linux-api@vger.kernel.org
>> Subject: [PATCH RFC 0/3] Drivers: hv: utils: re-implement the
>> kernel/userspace communication layer
>> 
>> This series converts kvp/vss daemons to use misc char devices instead of
>> netlink for userspace/kernel communication and then updates fcopy to be
>> consistent with kvp/vss.
>> 
>> Userspace/kernel communication via netlink has a number of issues:
>> - It is hard for userspace to figure out if the kernel part was loaded or not
>>   and this fact can change as there is a way to enable/disable the service from
>>   host side. Racy daemon startup is also a problem.
>> - When the userspace daemon restarts/dies kernel part doesn't receive a
>>   notification.
>> - Netlink communication is not stable under heavy load.
>> - ...
>> 
>> RFC: I'm a bit puzzled on how to split commits 1 and 2 avoiding breakages.
>> Commit 3 can definitely be split, however, it is consistent with commits 1 and
>> 2 at this moment and I'm not sure such split will simplify the review.
>> 
>> Vitaly Kuznetsov (3):
>>   Drivers: hv: kvp: convert userspace/kernel communication to using char
>>     device
>>   Drivers: hv: vss: convert userspace/kernel communication to using char
>>     device
>>   Drivers: hv: fcopy: make it consistent with vss/kvp
>
> Vitaly,
>
> Thank you for working on this. Before I give you detailed comments on your
> patches, I wanted to understand if the cost of maintaining compatibility was
> carefully considered. As a first step we could look at cleanly abstracting the 
> transport (between user level and the kernel) out of the kernel driver code 
> as well as the new daemon code. What are your thoughts on
> this. Version negotiation  is obviously key to maintaining
> compatibility. One of the  options we can explore is to continue to
> use netlink for version negotiation and for appropriate daemon versions, we could use
> the char device mechanism for transporting the payload.

Ok, I'll try making it backwards compatible (though I'd opt for
full migratiot to char devices one day and thus having negotiation
possible via the same device as well as via netlink for now).

>
> I like the new state machine you have defined and this is orthogonal to the transport
> options we have. You have sought feedback on how we can split up these changes into
> smaller patches. This is how I would proceed here:
>
> Patch(es) to clean up  the current code: 
> 	Patch(es) to clean up the state machine.
> 	Patch(es) to isolate the kernel/user transport
> Patch(es) to implement the new transport

Thanks, I'll proceed in this way.

>
> Regards,
>
> K. Y
>> 
>>  drivers/hv/hv_fcopy.c       | 395 +++++++++++++++++++++++++---------------
>> ---
>>  drivers/hv/hv_kvp.c         | 396 +++++++++++++++++++++++++++-------------
>> ----
>>  drivers/hv/hv_snapshot.c    | 335 +++++++++++++++++++++++++++---------
>> -
>>  include/uapi/linux/hyperv.h |  10 ++
>>  tools/hv/hv_fcopy_daemon.c  |  48 ++++--
>>  tools/hv/hv_kvp_daemon.c    | 187 ++++-----------------
>>  tools/hv/hv_vss_daemon.c    | 141 +++-------------
>>  7 files changed, 824 insertions(+), 688 deletions(-)
>> 
>> --
>> 1.9.3

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