From patchwork Fri Jul 29 15:31:46 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Felipe Franciosi X-Patchwork-Id: 9252747 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id BD90760757 for ; Fri, 29 Jul 2016 18:06:03 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B353F283FD for ; Fri, 29 Jul 2016 18:06:03 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A7F35283FF; Fri, 29 Jul 2016 18:06:03 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 7AD0E283FD for ; Fri, 29 Jul 2016 18:06:02 +0000 (UTC) Received: from localhost ([::1]:60916 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bTCAZ-0002f8-4P for patchwork-qemu-devel@patchwork.kernel.org; Fri, 29 Jul 2016 14:05:59 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57707) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bTCAH-0002f3-4N for qemu-devel@nongnu.org; Fri, 29 Jul 2016 14:05:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bTCAB-0007R7-K6 for qemu-devel@nongnu.org; Fri, 29 Jul 2016 14:05:41 -0400 Received: from mail-co1nam03on0098.outbound.protection.outlook.com ([104.47.40.98]:57365 helo=NAM03-CO1-obe.outbound.protection.outlook.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bTCAA-0007Qz-DM for qemu-devel@nongnu.org; Fri, 29 Jul 2016 14:05:35 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nutanix.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=tNxKNWpfiiGURe7F8z4AdDA1fclo1sLkmRRJJMS2xcY=; b=XtNgZK+SNxFt7qtQkPbzd1S+8fI31TJRHwSCHvDQDMWOGHck1PmjPY9ejhrysx7XbkRXkJyjULVNUV5bysdsicpXqIkOiTdaGM5eM34Ybe9e760bXzwaKl2JJfXLjnsZK+4ddXffowqMAql0cv0DOgWwCiuDhGkxfWQPWPJGlBU= Received: from BLUPR02MB083.namprd02.prod.outlook.com (10.255.189.21) by BLUPR02MB082.namprd02.prod.outlook.com (10.255.189.13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.549.15; Fri, 29 Jul 2016 15:31:46 +0000 Received: from BLUPR02MB083.namprd02.prod.outlook.com ([169.254.2.39]) by BLUPR02MB083.namprd02.prod.outlook.com ([169.254.2.39]) with mapi id 15.01.0549.016; Fri, 29 Jul 2016 15:31:47 +0000 From: Felipe Franciosi To: =?utf-8?B?TWFyYy1BbmRyw6kgTHVyZWF1?= Thread-Topic: [PATCH for-2.7 v5.1 1/2] vhost-user: Introduce a new protocol feature REPLY_ACK. Thread-Index: AQHR6J675jRV0m5uaEuv7oTUq3UeUqAvXX4AgAAt4wA= Date: Fri, 29 Jul 2016 15:31:46 +0000 Message-ID: <66527FAB-CA7E-4B55-82F5-D75769D9C665@nutanix.com> References: <1469689643-11556-1-git-send-email-saxenap.ltc@gmail.com> <1469689643-11556-2-git-send-email-saxenap.ltc@gmail.com> In-Reply-To: Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=felipe@nutanix.com; x-ms-exchange-messagesentrepresentingtype: 1 x-originating-ip: [86.26.35.229] x-ms-office365-filtering-correlation-id: 4bc7cfc1-080e-4b06-4e9c-08d3b7c5742d x-microsoft-exchange-diagnostics: 1; BLUPR02MB082; 6:Qs/ZBH3Rcd3c8h+Xk/r3z++k03fGdaa8XgEOQtRyWXslA/rRwIxobaL5LgnUezn0in5r9TeKedNvP20bC1TIucBgr+UBgAVsDnZRNAE6M49Q8cuJOjuQH4/ItNgtCC/q/nrdumpvApq+345CqVNyV14iHB1B7M678LRInLAtRkJyrKz+lyHNhiE+89dakcp0fqM21yNhS2Wt9knYcop0KncDVeaOKlRu0BLKkUqgHK8pYUCfK1yRhAEZnNPkQy6XqxWgkEQAeaackQp6Ua2MnDwOXPGrJ+f5UBL4CGCOhvc=; 5:X6aFtIP/l+hjnBfXulqkZQs3h80OMI56jvoJht6iyR9opUs46xH42pn2g1KzZsH6y3kWtf/6bGQDF3H0gn7w1RUyFUyNYqgVf62wqg19QwB1QZdBYxlZQWXR/Pi7RWKhXwVxNnrwADnGelgCwAyhXA==; 24:G8oMysQbRZDRGMXoYHhrkxc3V5n6PoRXgTiDhHOYw+X8pYEKBR/q3BFi1qNhQl0LjtqqYF9J3AlwHf5bSA60gn8XmT83a0P58tl040LcFmA=; 7:DHCo15anjkBebDertIdAMvX6rFlq08OTsPdTP30SRxO5XpEDkBi3lTpQbaG23riCWHaC7GkmDmG9Byq3jPbz6/kQ8tyTvuj9LIm70fnI4saJcfwbBbemGpLUzPzzWewO3QfSyFCe7KQwHYqe9qNmKfq4U9WN33lN4OmF3Dmva5aQsiqeyT6zonDFI9bdfJv+aolh0tUmPLVz6/sY8Hl3medF6D8lPy61FgdWaIiF16iUV1N84dfEBy10EiBlxF2W x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BLUPR02MB082; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(278428928389397)(52384705835673)(17755550239193); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001); SRVR:BLUPR02MB082; BCL:0; PCL:0; RULEID:; SRVR:BLUPR02MB082; x-forefront-prvs: 0018A2705B x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(7916002)(24454002)(199003)(377454003)(189002)(101416001)(7736002)(2906002)(19580405001)(19580395003)(86362001)(4326007)(3280700002)(4001430100002)(97736004)(102836003)(15975445007)(189998001)(10400500002)(76176999)(586003)(82746002)(8936002)(33656002)(7906003)(50986999)(2900100001)(16236675004)(83716003)(3846002)(54356999)(107886002)(6116002)(19273905006)(8676002)(122556002)(81166006)(2950100001)(87936001)(110136002)(19617315012)(66066001)(7846002)(106116001)(92566002)(106356001)(105586002)(68736007)(36756003)(3660700001)(5002640100001)(81156014)(99286002)(104396002)(563064011); DIR:OUT; SFP:1102; SCL:1; SRVR:BLUPR02MB082; H:BLUPR02MB083.namprd02.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; received-spf: None (protection.outlook.com: nutanix.com does not designate permitted sender hosts) spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: nutanix.com X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Jul 2016 15:31:46.9698 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: bb047546-786f-4de1-bd75-24e5b6f79043 X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLUPR02MB082 X-detected-operating-system: by eggs.gnu.org: Windows 7 or 8 [fuzzy] X-Received-From: 104.47.40.98 X-Content-Filtered-By: Mailman/MimeDel 2.1.21 Subject: Re: [Qemu-devel] [PATCH for-2.7 v5.1 1/2] vhost-user: Introduce a new protocol feature REPLY_ACK. X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Anil Kumar Boggarapu , Prerna Saxena , Prerna Saxena , "Michael S. Tsirkin" , QEMU , Felipe Franciosi Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP Heya, On 29 Jul 2016, at 13:47, Marc-André Lureau > wrote: Hi On Thu, Jul 28, 2016 at 11:07 AM, Prerna Saxena > wrote: From: Prerna Saxena > This introduces the VHOST_USER_PROTOCOL_F_REPLY_ACK. If negotiated, client applications should send a u64 payload in response to any message that contains the "need_reply" bit set on the message flags. Setting the payload to "zero" indicates the command finished successfully. Likewise, setting it to "non-zero" indicates an error. Currently implemented only for SET_MEM_TABLE. Signed-off-by: Prerna Saxena > --- docs/specs/vhost-user.txt | 26 ++++++++++++++++++++++++++ hw/virtio/vhost-user.c | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) uint32_t flags; uint32_t size; /* the following payload size */ union { @@ -158,6 +160,25 @@ fail: return -1; } +static int process_message_reply(struct vhost_dev *dev, + VhostUserRequest request) bad indentation +{ + VhostUserMsg msg; + + if (vhost_user_read(dev, &msg) < 0) { + return 0; return -1 + } + + if (msg.request != request) { + error_report("Received unexpected msg type." + "Expected %d received %d", + request, msg.request); bad indentation + return -1; + } + + return msg.payload.u64 ? -1 : 0; +} + static bool vhost_user_one_time_request(VhostUserRequest request) { switch (request) { @@ -239,11 +260,18 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev, int fds[VHOST_MEMORY_MAX_NREGIONS]; int i, fd; size_t fd_num = 0; + bool reply_supported = virtio_has_feature(dev->protocol_features, + VHOST_USER_PROTOCOL_F_REPLY_ACK); bad indentation + VhostUserMsg msg = { .request = VHOST_USER_SET_MEM_TABLE, .flags = VHOST_USER_VERSION, }; + if (reply_supported) { + msg.flags |= VHOST_USER_NEED_REPLY_MASK; + } + for (i = 0; i < dev->mem->nregions; ++i) { struct vhost_memory_region *reg = dev->mem->regions + i; ram_addr_t offset; @@ -277,6 +305,10 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev, vhost_user_write(dev, &msg, fds, fd_num); + if (reply_supported) { + return process_message_reply(dev, msg.request); + } + return 0; } -- 1.8.1.2 Other than that, looks good to me, with those fixes Reviewed-by: Marc-André Lureau > -- Marc-André Lureau diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt index 777c49c..54b5c8f 100644 --- a/docs/specs/vhost-user.txt +++ b/docs/specs/vhost-user.txt @@ -37,6 +37,8 @@ consists of 3 header fields and a payload: * Flags: 32-bit bit field: - Lower 2 bits are the version (currently 0x01) - Bit 2 is the reply flag - needs to be sent on each reply from the slave + - Bit 3 is the need_reply flag - see VHOST_USER_PROTOCOL_F_REPLY_ACK for + details. * Size - 32-bit size of the payload @@ -126,6 +128,8 @@ the ones that do: * VHOST_GET_VRING_BASE * VHOST_SET_LOG_BASE (if VHOST_USER_PROTOCOL_F_LOG_SHMFD) +[ Also see the section on REPLY_ACK protocol extension. ] + There are several messages that the master sends with file descriptors passed in the ancillary data: @@ -254,6 +258,7 @@ Protocol features #define VHOST_USER_PROTOCOL_F_MQ 0 #define VHOST_USER_PROTOCOL_F_LOG_SHMFD 1 #define VHOST_USER_PROTOCOL_F_RARP 2 +#define VHOST_USER_PROTOCOL_F_REPLY_ACK 3 Message types ------------- @@ -464,3 +469,24 @@ Message types is present in VHOST_USER_GET_PROTOCOL_FEATURES. The first 6 bytes of the payload contain the mac address of the guest to allow the vhost user backend to construct and broadcast the fake RARP. + +VHOST_USER_PROTOCOL_F_REPLY_ACK: +------------------------------- +The original vhost-user specification only demands replies for certain +commands. This differs from the vhost protocol implementation where commands +are sent over an ioctl() call and block until the client has completed. + +With this protocol extension negotiated, the sender (QEMU) can set the +"need_reply" [Bit 3] flag to any command. This indicates that +the client MUST respond with a Payload VhostUserMsg indicating success or +failure. The payload should be set to zero on success or non-zero on failure. +(Unless the message already has an explicit reply body) Unless/unless (for consistency, the rest of the document doesn't use Upper-case inside parentheses) Actually, if the sentence starts inside the parenthesis it should be capital. See rule 2a: http://www.grammarbook.com/punctuation/parens.asp Prerna's text looks correct to me. If it's wrong in other places we should probably fix it there separately. + +This indicates to QEMU that the requested operation has deterministically +been met or not. Today, QEMU is expected to terminate the main vhost-user +loop upon receiving such errors. In future, qemu could be taught to be more +resilient for selective requests. + +For the message types that already solicit a reply from the client, the +presence of VHOST_USER_PROTOCOL_F_REPLY_ACK or need_reply bit being set brings +no behaviourial change. (See the 'Communication' section for details.) See/see Same as my comment above. Cheers, Felipe diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 495e09f..86e7ae0 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -31,6 +31,7 @@ enum VhostUserProtocolFeature { VHOST_USER_PROTOCOL_F_MQ = 0, VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1, VHOST_USER_PROTOCOL_F_RARP = 2, + VHOST_USER_PROTOCOL_F_REPLY_ACK = 3, VHOST_USER_PROTOCOL_F_MAX }; @@ -84,6 +85,7 @@ typedef struct VhostUserMsg { #define VHOST_USER_VERSION_MASK (0x3) #define VHOST_USER_REPLY_MASK (0x1<<2) +#define VHOST_USER_NEED_REPLY_MASK (0x1 << 3) You could align it, and use the same style as the line above