Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753523AbdFVSi1 (ORCPT ); Thu, 22 Jun 2017 14:38:27 -0400 Received: from mail.kernel.org ([198.145.29.99]:59516 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751193AbdFVSiZ (ORCPT ); Thu, 22 Jun 2017 14:38:25 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C84A922B60 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=sstabellini@kernel.org Date: Thu, 22 Jun 2017 11:38:23 -0700 (PDT) From: Stefano Stabellini X-X-Sender: sstabellini@sstabellini-ThinkPad-X260 To: Andrew Cooper cc: Stefano Stabellini , =?UTF-8?Q?Roger_Pau_Monn=C3=A9?= , jgross@suse.com, linux-kernel@vger.kernel.org, xen-devel@lists.xen.org, Stefano Stabellini , boris.ostrovsky@oracle.com Subject: Re: [Xen-devel] [PATCH v4 07/18] xen/pvcalls: implement socket command In-Reply-To: <0ae24b76-03f5-5adf-5437-2c61f934502b@citrix.com> Message-ID: References: <1497553787-3709-1-git-send-email-sstabellini@kernel.org> <1497553787-3709-7-git-send-email-sstabellini@kernel.org> <20170620161112.7x4qickboyt7qyvh@dhcp-3-128.uk.xensource.com> <20170622082632.atg6dslz5zs2gtco@dhcp-3-128.uk.xensource.com> <0ae24b76-03f5-5adf-5437-2c61f934502b@citrix.com> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: MULTIPART/MIXED; BOUNDARY="8323329-338633432-1498156705=:12819" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4972 Lines: 107 This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --8323329-338633432-1498156705=:12819 Content-Type: TEXT/PLAIN; charset=UTF-8 Content-Transfer-Encoding: 8BIT On Thu, 22 Jun 2017, Andrew Cooper wrote: > On 22/06/17 19:29, Stefano Stabellini wrote: > > On Thu, 22 Jun 2017, Roger Pau Monné wrote: > >> On Wed, Jun 21, 2017 at 01:16:56PM -0700, Stefano Stabellini wrote: > >>> On Tue, 20 Jun 2017, Roger Pau Monné wrote: > >>>> On Thu, Jun 15, 2017 at 12:09:36PM -0700, Stefano Stabellini wrote: > >>>>> Just reply with success to the other end for now. Delay the allocation > >>>>> of the actual socket to bind and/or connect. > >>>>> > >>>>> Signed-off-by: Stefano Stabellini > >>>>> CC: boris.ostrovsky@oracle.com > >>>>> CC: jgross@suse.com > >>>>> --- > >>>>> drivers/xen/pvcalls-back.c | 27 +++++++++++++++++++++++++++ > >>>>> 1 file changed, 27 insertions(+) > >>>>> > >>>>> diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c > >>>>> index 437c2ad..953458b 100644 > >>>>> --- a/drivers/xen/pvcalls-back.c > >>>>> +++ b/drivers/xen/pvcalls-back.c > >>>>> @@ -12,12 +12,17 @@ > >>>>> * GNU General Public License for more details. > >>>>> */ > >>>>> > >>>>> +#include > >>>>> #include > >>>>> #include > >>>>> #include > >>>>> #include > >>>>> #include > >>>>> #include > >>>>> +#include > >>>>> +#include > >>>>> +#include > >>>>> +#include > >>>>> > >>>>> #include > >>>>> #include > >>>>> @@ -54,6 +59,28 @@ struct pvcalls_fedata { > >>>>> static int pvcalls_back_socket(struct xenbus_device *dev, > >>>>> struct xen_pvcalls_request *req) > >>>>> { > >>>>> + struct pvcalls_fedata *fedata; > >>>>> + int ret; > >>>>> + struct xen_pvcalls_response *rsp; > >>>>> + > >>>>> + fedata = dev_get_drvdata(&dev->dev); > >>>>> + > >>>>> + if (req->u.socket.domain != AF_INET || > >>>>> + req->u.socket.type != SOCK_STREAM || > >>>>> + (req->u.socket.protocol != IPPROTO_IP && > >>>>> + req->u.socket.protocol != AF_INET)) > >>>>> + ret = -EAFNOSUPPORT; > >>>> Sorry for jumping into this out of the blue, but shouldn't all the > >>>> constants used above be part of the protocol? AF_INET/SOCK_STREAM/... > >>>> are all part of POSIX, but their specific value is not defined in the > >>>> standard, hence we should have XEN_AF_INET/XEN_SOCK_STREAM/... Or am I > >>>> just missing something? > >>> The values of these constants for the pvcalls protocol are defined by > >>> docs/misc/pvcalls.markdown under "Socket families and address format". > >>> > >>> They happen to be the same as the ones defined by Linux as AF_INET, > >>> SOCK_STREAM, etc, so in Linux I am just using those, but that is just an > >>> implementation detail internal to the Linux kernel driver. What is > >>> important from the protocol ABI perspective are the values defined by > >>> docs/misc/pvcalls.markdown. > >> Oh I see. I still think this should be part of the public pvcalls.h > >> header, and that the error codes should be the ones defined in > >> public/errno.h (or else also added to the pvcalls header). > > This was done differently in the past, but now that we have a formal > > process, a person in charge of new PV drivers reviews, and design > > documents with clearly spelled out ABIs, I consider the design docs > > under docs/misc as the official specification. We don't need headers > > anymore, they are redundant. In fact, we cannot have two specifications, > > and the design docs are certainly the official ones (we don't want the > > specs to be written as header files in C). To me, the headers under > > xen/include/public/io/ are optional helpers. It doesn't matter what's in > > there, or if frontends and backends use them or not. > > > > There is really an argument for removing those headers, because they > > might get out of sync with the spec by mistake, and in those cases, then > > we really end up with two specifications for the same protocol. I would > > be in favor of `git rm'ing all files under xen/include/public/io/ for > > which we have a complete design doc under docs/misc. > > +1. > > Specifications should not be written in C. The mess that is the net and > block protocol ABIs are perfect examples of why. > > Its fine (and indeed recommended) to provide a header file which > describes the specified protocol, but the authoritative spec should be > in text from. > > I would really prefer if more people started using ../docs/specs/. The > migration v2 documents are currently lonely there... I didn't realize we had a docs/specs. Feel free to move pvcalls and 9pfs under there. --8323329-338633432-1498156705=:12819--