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

[KY Srinivasan]

> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@xxxxxxxxxx]
> Sent: Friday, February 27, 2015 8:14 AM
> To: KY Srinivasan; devel@xxxxxxxxxxxxxxxxxxxxxx
> Cc: Haiyang Zhang; linux-kernel@xxxxxxxxxxxxxxx; Dexuan Cui; Radim Krčmář;
> Greg Kroah-Hartman; linux-api@xxxxxxxxxxxxxxx
> 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.

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

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